fix(forms): render public form embeds via SSR plugin routes#985
fix(forms): render public form embeds via SSR plugin routes#985ppppangu wants to merge 3 commits into
Conversation
🦋 Changeset detectedLatest commit: ec13951 The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/blocks
@emdash-cms/cloudflare
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
|
/review |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Targeted fix for the Workers SSR self-fetch issue, with the wrapped helper correctly gated on getPluginRouteMeta(...).public === true and decent unit coverage on both sides. Two things worth addressing before merge — the silent fallback when the dispatcher throws hides exactly the failures this PR is meant to solve, and Astro.locals.emdash.handlePluginApiRoute now has materially different semantics on the anonymous vs. full middleware paths (public-only vs. raw runtime). A couple of smaller robustness notes inline as well. Nothing here is a blocker.
|
Review posted successfully (ID 4263518046, 7 inline comments). The review covers:
|
|
This PR has been inactive for 14 days. It will be closed automatically in 7 days if there is no further activity. If you're still working on this, please push an update or leave a comment. |
ppppangu
force-pushed
the
fix-public-plugin-routes-ssr
branch
from
42588b0 to
ec13951
Compare
|
@ascorbic All 7 review threads have been addressed and resolved, the branch has been rebased, and all CI checks are green. Could you remove the stale “Awaiting author response” label? Thanks! |
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
2 participants
What does this PR do?
Fixes public form embeds during SSR by giving anonymous public pages a restricted way to call public plugin routes. Form embeds now resolve their public definition through the core dispatcher when available, which avoids server-side self-fetching in Worker SSR while preserving the old fetch path for older runtimes or dispatcher failures.
The core helper gates every internal dispatch on
getPluginRouteMeta(...).public === true, so private plugin routes still require the normal API route auth, scope, and CSRF path. The forms plugin only calls the fixedemdash-forms/definitionroute and continues to parse the existing{ data: ... }API envelope.Closes: none
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs; a workflow extracts catalogs on merge tomain.AI-generated code disclosure
Screenshots / test output
No screenshots; this is an SSR/plugin route bug fix.
Verification run:
pnpm --filter emdash test -- public-plugin-api-routes middleware-prerenderpassedpnpm --filter @emdash-cms/plugin-forms test -- public-definitionpassedpnpm --filter @emdash-cms/plugin-forms typecheckpassedpnpm --filter emdash buildpassedpnpm typecheckpassedpnpm lintexited with 0 errors; it reports existing warnings outside this changepnpm exec prettier --checkon the touched files passedNotes:
pnpm formatbecausepnpm format:checkreports broad existing formatting drift in this Windows checkout; I kept formatting validation scoped to touched files to avoid unrelated churn.pnpm --filter emdash testcurrently has unrelated Windows-environment failures intests/unit/astro/vite-config.test.ts(File URL path must be absolute) andtests/unit/cli/bundle-utils.test.ts(systemtarlisting CRLF). The targeted core tests for this change pass.